Add ASN numbers as scan targets#2741
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # Test ASN with other targets | ||
| target = BBOTTarget("ASN:15169", "evilcorp.com", "1.2.3.4/24") | ||
| assert "ASN:15169" in target.seeds.inputs | ||
| assert "evilcorp.com" in target.seeds.inputs |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
The code currently checks for the presence of "evilcorp.com" as a substring in target.seeds.inputs, which is subject to the vulnerabilities described. To fix this, the test should specifically check if "evilcorp.com" is present as an actual host, not as a substring within any input string. This can be done by parsing the inputs and extracting their hostnames for comparison, or, if target.seeds provides a .hosts attribute containing canonicalized hosts, use that for the assertion. Thus, change the assertion on line 377 to ensure "evilcorp.com" is in the normalized or parsed hosts. This may require referencing the hosts set or parsing the inputs using a standard library (e.g., urlparse).
Make the following changes in bbot/test/test_step_1/test_target.py:
- On line 377, replace the substring-in check with an explicit hostname check using either the
.hostsattribute (if available), or by parsing input values usingurlparse. - If using
urlparse, import it from the standard library.
| @@ -374,7 +374,7 @@ | ||
| # Test ASN with other targets | ||
| target = BBOTTarget("ASN:15169", "evilcorp.com", "1.2.3.4/24") | ||
| assert "ASN:15169" in target.seeds.inputs | ||
| assert "evilcorp.com" in target.seeds.inputs | ||
| assert "evilcorp.com" in [str(host) for host in target.seeds.hosts] | ||
| assert "1.2.3.0/24" in target.seeds.inputs # IP ranges are normalized to network address | ||
|
|
||
| # Test ASN targets must be expanded before being useful in whitelist/blacklist |
📊 Performance Benchmark Report
📈 Detailed Results (All Benchmarks)
🎯 Performance Summary+ 2 improvements 🚀
18 unchanged ✅🔍 Significant Changes (>10%)
🐍 Python Version 3.11.15 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 3.0 #2741 +/- ##
======================================
- Coverage 92% 91% -0%
======================================
Files 436 434 -2
Lines 36320 36452 +132
======================================
+ Hits 33059 33141 +82
- Misses 3261 3311 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This PR is currently waiting on two things:
|
liquidsec
force-pushed
the
asn-as-targets
branch
from
4dcc8ba to
a65a347
Compare
liquidsec
changed the title
liquidsec
force-pushed
the
asn-as-targets
branch
from
a65a347 to
344130e
Compare
bbot/core/helpers/misc.py
Outdated
| async def as_completed(coros): | ||
| async def as_completed( | ||
| coroutines: Iterable[Awaitable], | ||
| max_concurrent: Optional[int] = None, |
There was a problem hiding this comment.
Default limit should be something reasonable like 20-30 etc.
There was a problem hiding this comment.
this still applies correct?
bbot/core/modules.py
Outdated
| raise BBOTError( | ||
| log.warning( | ||
| f"Error loading module {module_name}: {e}. You may have leftover artifacts from an older version of BBOT. Try deleting/renaming your '~/.bbot' directory." | ||
| ) from e | ||
| ) | ||
| module = None |
There was a problem hiding this comment.
The error was intentional to stop the scan if a module failed to load
bbot/scanner/preset/preset.py
Outdated
| if self._target is None: | ||
| raise ValueError("Cannot access target before preset is baked (use ._seeds instead)") | ||
| return self._target | ||
|
|
||
| @property | ||
| def seeds(self): | ||
| if self._target is None: | ||
| raise ValueError("Cannot access target before preset is baked (use ._seeds instead)") | ||
| return None | ||
| return self.target.seeds | ||
|
|
||
| @property | ||
| def blacklist(self): | ||
| if self._target is None: | ||
| raise ValueError("Cannot access blacklist before preset is baked (use ._blacklist instead)") | ||
| return None |
There was a problem hiding this comment.
these errors are designed to catch situations where these attributes are accessed prematurely (i.e. before bake). if they're returning None, we shouldn't need to access them
let's put it back and see what breaks
bbot/scanner/scanner.py
Outdated
| j["target"] = self.preset.target.json | ||
| j["preset"] = self.preset.to_dict(redact_secrets=True) | ||
| if self.preset is not None: | ||
| j["target"] = self.preset.target.json | ||
| j["preset"] = self.preset.to_dict(redact_secrets=True) | ||
| else: | ||
| j["target"] = {} | ||
| j["preset"] = {} |
There was a problem hiding this comment.
can we try reverting this to see why it had to be introduced? a scan should never exist without a preset (it should be impossible)
There was a problem hiding this comment.
after migrating to asndb pypi library, passing in helpers shouldn't be necessary
bbot/core/helpers/command.py
Outdated
| kwargs["stdin"] = asyncio.subprocess.PIPE | ||
|
|
||
| log.hugeverbose(f"run: {' '.join(command)}") | ||
| log.debug(f"run: {' '.join(command)}") |
There was a problem hiding this comment.
we must show commands when -v. if necessary, we can add a exclusion flag
- BaseTarget no longer subclasses RadixTarget; uses composition instead - Rename strict_dns_scope -> strict_scope everywhere - Update host_size_key import path for radixtarget 4.x - Handle radixtarget 4.x API changes (strings-only, no _add, hash is int) - Skip acl_mode when strict_scope is True (mutually exclusive in 4.x) - Update test assertions for new hash values and string-based hosts Work in progress - more test fixes needed.
# Conflicts: # bbot/scanner/scanner.py # bbot/test/test_step_1/test_python_api.py
…cope filtering - Remove ASN event emission (already handled by asn report module via asndb) - Replace scope_distance_modifier+filter_event with proper in_scope_only class attr - Dynamically set scope_distance_modifier=1 when in_scope_only option is disabled - Add tests for in_scope_only=True and in_scope_only=False behavior
…ibrary Migrate to asndb library
bbot/scanner/target.py
Outdated
| # After expanding seeds, update the target to include any new hosts from seed expansion | ||
| # This ensures that expanded targets (like IP ranges from ASN) are considered in-scope | ||
| # BUT only if no custom target was provided - don't override user's custom target | ||
| if not had_custom_target: | ||
| expanded_seed_hosts = set(self.seeds.hosts) | ||
| for host in expanded_seed_hosts: | ||
| if host not in self.target: | ||
| self.target.add(host) |
There was a problem hiding this comment.
Isn't a target by nature custom? I.e. I don't think it's possible to specify ASNs without a custom target? It seems like this should always happen. What do you think
There was a problem hiding this comment.
I think the logic is correct (user can assign a more specific/explicit target than the seeds) but the naming made it confusing. Updated variable names/comments to alleviate.
| asn_helper = module_test.scan.helpers.asn | ||
| asn_helper._client = AsyncMock() | ||
| asn_helper._client.lookup_ip = AsyncMock(return_value=self.asndb_response) | ||
| asn_helper._client.lookup_asn = AsyncMock(return_value=self.asndb_response) |
There was a problem hiding this comment.
Instead of using asyncmock, we should use the real thing. we can hardcode 1.1.1.1 and 8.8.8.8, etc.
There was a problem hiding this comment.
I agree, but the problem is the google ASN is ENORMOUS and the test takes too long and times out
There was a problem hiding this comment.
and the purpose of the test is to test the module - not the asn behavior itself
There was a problem hiding this comment.
nevermind there was a big in the tests which i fixed. removed the mock
| # Ensure stdout/stderr are blocking before pytest writes summaries | ||
| try: | ||
| import sys | ||
| import fcntl | ||
| import os | ||
| import io | ||
|
|
||
| fds = [] | ||
| for stream in (sys.stdout, sys.stderr): | ||
| try: | ||
| fds.append(stream.fileno()) | ||
| except io.UnsupportedOperation: | ||
| pass | ||
| for fd in fds: | ||
| flags = fcntl.fcntl(fd, fcntl.F_GETFL) | ||
| fcntl.fcntl(fd, fcntl.F_SETFL, flags & ~os.O_NONBLOCK) | ||
| except Exception: | ||
| pass | ||
|
|
There was a problem hiding this comment.
was this to address a specific bug?
There was a problem hiding this comment.
fixes a bug where pytest crashes during test summary output with BlockingIOError: [Errno 11] write could not complete without blocking.
From claude:
BBOT's engine subprocesses set stdout/stderr to non-blocking mode (via asyncio). If a subprocess doesn't clean up properly on shutdown, the file descriptors stay non-blocking. When pytest then tries to write its
summary (which can be large), the write fails because the fd can't accept all the data at once in non-blocking mode.
It was showing up as flaky test failures where all tests pass but pytest itself crashes at the end trying to print results.
2 participants
Summary
AS13335) as scan targets. They are automatically expanded into their constituent IP ranges before the scan begins.asndblibrary: ASN lookups now use theasndblibrary instead of hand-rolled API calls. This handles ASN→CIDR expansion, IP→ASN reverse lookups, and caching.radixtarget4.x:BBOTTargetnow uses composition (owns aRadixTarget) instead of inheritance. This aligns with the radixtarget 4.x string-only API.BBOTTarget.generate_children(): New async method on the target object that expands compound seed types (like ASNs) into child events before the scan begins.shodan_enterprisemodule cleanup: Removed redundant ASN emission (handled byasnreport module via asndb). Fixed scope filtering to use framework'sin_scope_onlyattribute properly.install_core_depsperformance: Core dependency installation now caches properly, reducing redundant 15s Ansible invocations to ~0s on subsequent runs.test_python_apipreset checks and scope accuracy tests. Added scope filtering tests forshodan_enterprise. Removed mocks in favor of realasndblookups.